Building a Cloud CLI with Bun for API communication#4480
Building a Cloud CLI with Bun for API communication#4480
Conversation
Co-authored-by: jog1t <39823706+jog1t@users.noreply.github.com> Agent-Logs-Url: https://github.com/rivet-dev/rivet/sessions/2d03d5a3-64ac-44aa-b359-041b30a15992
…client Replace the hand-rolled CloudClient with the @rivet-gg/cloud SDK that is already installed in the monorepo: - client.ts now exports a createCloudClient() factory wrapping RivetClient - deploy.ts uses client.apiTokens.inspect(), client.namespaces.get/create(), and client.managedPools.upsert() from the SDK; drops the unreleased getDockerCredentials() endpoint in favour of always using token-based registry auth (registry.rivet.dev accepts cloud tokens directly) - logs.ts uses RivetSse.streamLogs() from the SDK, mirroring the frontend's use-deployment-logs-stream.ts; handles connected/end/error/log events - tests updated to test the SDK-based client rather than the old fetch mocks - args field passed to managedPools.upsert() as string[] (split on space) to match the SDK type Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR Review: Cloud CLI with BunThis PR adds a new cloud-cli package (Bun-based CLI for deploying Docker images and streaming logs to Rivet Cloud). The overall structure is reasonable, but there are several issues to address before merging. Critical Issues 1. Variable scope bug in deploy.ts (runtime crash): identity, namespace, managedPool, and creds are all declared inside the task callback closure but referenced outside of it after the task resolves. This causes a ReferenceError at runtime. These variables need to be hoisted before the outer task, or returned from it. 2. Wrong dashboard URL: The success output in deploy.ts uses dashboard.rivet.dev. Per CLAUDE.md the correct URL is hub.rivet.dev. 3. @rivet-gg/cloud package name: package.json depends on @rivet-gg/cloud. CLAUDE.md says to never use rivet-gg/* packages. If this is the canonical npm package name, add a comment explaining the exception. 4. PR preview registry in lockfile: The lockfile pins @rivet-gg/cloud to a pkg.pr.new CI preview artifact URL. This must be replaced with the official npm version before merging. Bugs 5. Silent pool error in deployment wait: In the Deploying polling loop, when pool.error is truthy the loop breaks silently without reporting anything to the user. fatal() should be called with the error details instead. 6. Unbounded polling loops: Both while(true) polling loops have no timeout or maximum iteration count. A pool stuck in a non-terminal state will loop forever. 7. Temp file never cleaned up: dockerBuild creates /tmp/rivet-cloud-cli-iid-N but never deletes it. Add a finally block to remove the file after reading it. Minor Issues 8. No validation for --min-count or --max-count. Number with a non-numeric string silently returns NaN. 9. opts.args.split breaks on quoted args containing spaces. Document the limitation or switch to a repeatable flag pattern. 10. Log reconnect backoff is uncapped. Consider capping around 30s. 11. RivetClient is constructed with environment set to empty string, which looks like a leftover placeholder. Verify or remove. 12. Several JSDoc comments use em dashes which CLAUDE.md explicitly forbids. 13. client.test.ts mostly checks instanceof RivetClient. Extend the mock-fetch tests to assert that tokens and base URLs are forwarded correctly. Summary
Issues 1 and 5 are the most urgent fixes before this can be considered correct. |
Pull request created by AI Agent